Video Store _ Christina & Maryam by marshi23 · Pull Request #24 · Ada-C10/VideoStoreAPI

marshi23 · 2018-11-10T02:25:54Z

Video Store API

Congratulations! You're submitting your assignment!
If you didn't get to the functionality the question is asking about, reply with what you would have done if you had completed it.

Comprehension Questions

Question	Answer
Explain how you came up with the design of your ERD, based on the seed data.	The seed data was helpful to know what data we needed to input to our tables and columns.
What would be the Big-O time complexity of your /customers & /movies endpoints? What does the time complexity depend on? Explain your reasoning.	O(n) because it depends on the length of customers or movies in the db.
What is the Big-O time complexity of the `POST /rentals/check-in` endpoint? What does the time complexity depend on? Explain your reasoning.	Constant because we're returning a JSON response that it was successful or unsuccessful.
Describe a set of positive and negative test cases you implemented for a model.	we tested for the validity of a movie given the required fields and then tested it without the required fields
Describe a set of positive and negative test cases you implemented for a controller.	In our rental controller we test that the check-out path works with a valid input params and one without.
How does your API respond when bad data is sent to it?	It responds with a 404 bad request status
Describe one of your custom model methods and why you chose to wrap that functionality into a method.	We have a checkout! class method in our rental model that takes in movie and customer ids. It adjusts and creates a rental object, makes note of the date, assigns a return date, updates the movie's inventory and the customer's movies_checked_out count.
Do you have any recommendations on how we could improve this project for the next cohort?	Do this before API muncher, explain using postman better in relations to creating custom APIs
Link to Trello	https://trello.com/b/RZ6m8bHB
Link to ERD	https://www.lucidchart.com/invitations/accept/7b052de5-82d0-49e6-bb27-bba6572e18f8
Summary	We feel like we learned alot on this project, especially because we had enough project time to understand the concepts.

saving merge

CheezItMan · 2018-11-11T21:17:12Z

Video Store

What We're Looking For

Feature	Feedback
Core Requirements
Git hygiene	Good number of commits and good commit messages
Comprehension questions	Check, I would say O(n) for checkin and check-out because you have to find the customer and movie. You also should not return 404, for invalid data. Instead a bad request would make an appropriate response in some cases.
General
Business Logic in Models	Check, but you could have more in the customer and movie models.
All required endpoints return expected JSON data	Check
Requests respond with appropriate HTTP Status Code	Check
Errors are reported	Not always, see my inline notes
Testing
Passes all Smoke Tests	You're not passing tests revolving around available inventory, and the movies checked out.
Model Tests - all relations, validations, and custom functions test positive & negative cases	You do not have tests for your relationships or custom methods in `Rental`. Your testing is pretty thin here as well.
Controller Tests - URI parameters and data in the request body have positive & negative cases	You need to test negative cases here. Like creating a movie with invalid parameters.
Overall	You hit most of the learning goals here. You need to do a better job of testing throughly, adding more business logic in the models, and better matching the smoke tests.

CheezItMan · 2018-11-11T21:09:56Z

app/controllers/movies_controller.rb

+
+    def create
+      @movie = Movie.new(movie_params)
+      @movie.save!


You should return a json response if the move wasn't able to be saved. Provide error messages.

CheezItMan · 2018-11-11T21:10:38Z

app/controllers/rentals_controller.rb

+      rental = Rental.checkout!(rental_params[:customer_id], rental_params[:movie_id])
+      render json: { ok: true, message: 'Checkout successful!' }, status: :ok
+    else
+      render json: { ok: false, message: 'Unable to checkout' },


This wouldn't be a not found but rather a bad request.

CheezItMan · 2018-11-11T21:11:32Z

app/controllers/rentals_controller.rb

+    end
+
+  rescue ArgumentError
+    render json: { ok: false, message: 'Unable to checkout: movie or customer does not exist' }, status: :not_found


Again, not_found is probably not the right response here.

CheezItMan · 2018-11-11T21:12:58Z

test/models/movie_test.rb

+
+    it "has rentals" do
+
+       movie.rentals.new


What is this line trying to accomplish?

CheezItMan · 2018-11-11T21:13:18Z

test/models/movie_test.rb

+
+       movie.rentals.new
+
+      expect (movie.rentals.length).must_be :>=, 1


I would also add a test that rentals is a collection of Rental instances.

CheezItMan · 2018-11-11T21:16:12Z

test/controllers/movies_controller_test.rb

+
+    end
+
+    it 'creates a movie given correct params' do


negative cases?

CheezItMan · 2018-11-11T21:16:27Z

test/controllers/rentals_controller_test.rb

+      }
+
+      post rentals_checkout_path(rental.id), {:params => rental_params}
+      rental.reload


You also need negative cases as well.

marshi23 and others added 30 commits November 5, 2018 11:28

rails app initiated

ea894fa

models created

2fff523

added foreign keys migration

f1ae420

added foreign keys

2967049

validations added

c8ce3d6

added two customer model tests, both passing

1b53b13

added third customer model test

cf68a41

movie tests added

cb55128

testing

e6ce34a

created controllers

5f17609

movie model test done

edc5eda

Merge branch 'master' of https://github.com/marshi23/VideoStoreAPI

ac5a4d5

added show method, works

853030b

returning customers working

236d480

merging

9c86988

added two controller tests, both passing

e0b3dd5

three controller tests, all passing

394c72f

seven tests in movie controller, all passing

3133cea

customer controller tests added, checkout added

8a6a67b

Merge branch 'master' of https://github.com/marshi23/VideoStoreAPI

76ada12

checking updates

8080a96

checkout function working

a8188ae

all movie controller tests passing

2ac4e38

all movie controller tests passing

0d4d47d

Merge branch 'master' of https://github.com/marshi23/VideoStoreAPI

59f0ffe

customers movie count bug fix

a7f3786

all tests passing, except smoke avail_inventory

b68c5ec

Merge branch 'master' of https://github.com/marshi23/VideoStoreAPI

7f99d38

saving merge

all tests passing, including smoke

0972807

added one rental test, passing

aa5145f

marshi23 and others added 19 commits November 7, 2018 13:56

rental model test for checkout action added

72e8e0f

Merge branch 'master' of https://github.com/marshi23/VideoStoreAPI

2c629f2

fixing tests that broke, one or two left

80d5e5a

fixed test, all tests passing

950c4ee

sorting out checkin method

1555d09

rental test added"

f1948c8

merging versions

2778034

space cleanup

e0f4ffc

merging files

5d352e3

merging

866bb84

adding test

7160f0d

checkin controller action added

e167b1a

pushing up changes

62cf4be

rental controller checkin update

6a22ddb

checkin path test working

2473886

tests passing

b663200

changing tests and amending code

ff383b0

all tests passing properly

38e7317

need to add expectation to rentals controller inventory

cfd6136

CheezItMan reviewed Nov 11, 2018

View reviewed changes

fixed last test, all green

88677a6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Video Store _ Christina & Maryam#24

Video Store _ Christina & Maryam#24
marshi23 wants to merge 50 commits intoAda-C10:masterfrom
marshi23:master

marshi23 commented Nov 10, 2018

Uh oh!

CheezItMan commented Nov 11, 2018

Uh oh!

CheezItMan Nov 11, 2018

Uh oh!

CheezItMan Nov 11, 2018

Uh oh!

CheezItMan Nov 11, 2018

Uh oh!

CheezItMan Nov 11, 2018

Uh oh!

CheezItMan Nov 11, 2018

Uh oh!

CheezItMan Nov 11, 2018

Uh oh!

CheezItMan Nov 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		movie.rentals.new

		expect (movie.rentals.length).must_be :>=, 1

Conversation

marshi23 commented Nov 10, 2018

Video Store API

Comprehension Questions

Uh oh!

CheezItMan commented Nov 11, 2018

Video Store

What We're Looking For

Uh oh!

CheezItMan Nov 11, 2018

Choose a reason for hiding this comment

Uh oh!

CheezItMan Nov 11, 2018

Choose a reason for hiding this comment

Uh oh!

CheezItMan Nov 11, 2018

Choose a reason for hiding this comment

Uh oh!

CheezItMan Nov 11, 2018

Choose a reason for hiding this comment

Uh oh!

CheezItMan Nov 11, 2018

Choose a reason for hiding this comment

Uh oh!

CheezItMan Nov 11, 2018

Choose a reason for hiding this comment

Uh oh!

CheezItMan Nov 11, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants